Skip to content

Add wpdb fallback for wp db query and wp db import when mysql/mariadb binary is unavailable#320

Open
swissspidy with Copilot wants to merge 25 commits into
mainfrom
copilot/fix-wp-db-query-fallback
Open

Add wpdb fallback for wp db query and wp db import when mysql/mariadb binary is unavailable#320
swissspidy with Copilot wants to merge 25 commits into
mainfrom
copilot/fix-wp-db-query-fallback

Conversation

Copilot AI commented Apr 2, 2026

Copy link
Copy Markdown
Contributor

wp db query and wp db import currently require the mysql/mariadb CLI binary, causing failures with drop-in database engines (HyperDB, custom drivers) or environments where the binary isn't installed. This PR adds automatic fallback to WordPress's $wpdb for these commands when the binary is unavailable.

Changes

src/DB_Command.php

  • is_mysql_binary_available() — probes for the mysql/mariadb binary via Process::create(); result is statically cached.
  • maybe_load_wpdb() — loads minimal WordPress files needed for $wpdb (load.php, compat.php, plugin.php, functions.php, class-wpdb.php), includes any wp-content/db.php drop-in (e.g. HyperDB), and creates a plain wpdb instance with wp-config.php credentials and $table_prefix if $wpdb is not already initialized.
  • query() — checks binary availability after SQLite handling; routes to wpdb_query() when absent.
  • wpdb_query() — executes SQL via $wpdb; formats SELECT results as tab-separated values matching the mysql CLI output format, and reports affected rows for DML operations.
  • import() — when the mysql binary is unavailable, reads SQL from file or STDIN and delegates to wpdb_import().
  • wpdb_import() — executes SQL statement-by-statement through $wpdb with transaction and constraint optimization toggles (autocommit=0, unique_checks=0, foreign_key_checks=0), suppressing $wpdb output and ignoring non-essential privilege errors on SET statements (e.g. SQL_LOG_BIN, GTID).
  • split_sql_statements() — state-machine SQL parser handling single/double-quoted strings, line comments (--), block comments (/* */), and MySQL conditional comments (/*!...*/).

Tests

  • Added @require-mysql-or-mariadb fallback scenarios in features/db-query.feature and features/db-import.feature asserting fallback to $wpdb when binaries are shadowed.

Note: Server-level DDL commands (wp db create, wp db drop, wp db reset) are not covered by the $wpdb fallback because $wpdb operates within an established connection to a specific database context (DB_NAME).

Summary by CodeRabbit

  • New Features

    • wp db query now falls back to WordPress database execution when MySQL/MariaDB client tools are unavailable.
    • wp db import can import SQL files or standard input without requiring MySQL/MariaDB client binaries.
    • Fallback operations provide compatible query results, import confirmation, and diagnostic warnings.
  • Bug Fixes

    • Improved SQL handling for quoted text, comments, and conditional statements during fallback execution.
    • Added clearer errors for missing or unreadable import files and empty queries.

@codecov

codecov Bot commented Apr 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 78.57143% with 39 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/DB_Command.php 78.57% 39 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copilot AI changed the title [WIP] Fix wp db query to fallback using WPDB Add wpdb fallback for wp db query when mysql/mariadb binary is unavailable Apr 2, 2026
Copilot AI requested a review from swissspidy April 2, 2026 21:31
@swissspidy

This comment was marked as resolved.

This comment was marked as resolved.

Copilot AI changed the title Add wpdb fallback for wp db query when mysql/mariadb binary is unavailable Add wpdb fallback for wp db query, wp db import, wp db create, wp db drop, and wp db reset when mysql/mariadb binary is unavailable Apr 3, 2026
Comment thread features/db-query.feature Outdated
@swissspidy

This comment was marked as resolved.

This comment was marked as resolved.

Copilot AI changed the title Add wpdb fallback for wp db query, wp db import, wp db create, wp db drop, and wp db reset when mysql/mariadb binary is unavailable Add wpdb fallback for wp db query, wp db import, wp db drop, and wp db reset when mysql/mariadb binary is unavailable Apr 3, 2026
Copilot AI requested a review from swissspidy April 3, 2026 09:12
Comment thread features/db.feature Outdated
…query/import

Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>

This comment was marked as resolved.

@swissspidy

This comment was marked as resolved.

Copilot AI and others added 2 commits July 14, 2026 11:34
…_import()

Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
…y output

Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>

This comment was marked as resolved.

@swissspidy

This comment was marked as resolved.

Copilot AI and others added 2 commits July 14, 2026 14:29
…...*/)

Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>
…p comments

Co-authored-by: swissspidy <841956+swissspidy@users.noreply.github.com>

This comment was marked as resolved.

This comment was marked as resolved.

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

wp db query and wp db import now detect unavailable MySQL/MariaDB binaries and execute through wpdb, including SQL parsing, output handling, error handling, and regression scenarios.

Changes

MySQL CLI fallback

Layer / File(s) Summary
Query fallback and output handling
src/DB_Command.php, features/db-query.feature, features/db.feature
Queries fall back to wpdb when the CLI binary is unavailable, preserve tabular output behavior, and validate the fallback with a tagged scenario.
SQL import fallback and statement parsing
src/DB_Command.php, features/db-import.feature
Imports read SQL from files or STDIN, split statements while respecting quotes and comments, execute through wpdb, handle errors and optimization settings, and validate successful fallback imports.

Estimated code review effort: 4 (Complex) | ~45 minutes

Suggested labels: command:db, command:db-import

Suggested reviewers: swissspidy, schlessera, towytowy

Sequence Diagram(s)

sequenceDiagram
  participant WPCLI
  participant DB_Command
  participant wpdb
  participant Database
  WPCLI->>DB_Command: run db query or db import
  DB_Command->>DB_Command: detect missing MySQL/MariaDB binary
  DB_Command->>wpdb: execute query or split import statements
  wpdb->>Database: send SQL
  Database-->>wpdb: return rows, status, or errors
  wpdb-->>DB_Command: return execution results
  DB_Command-->>WPCLI: print output and fallback warning
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: adding wpdb fallback for wp db query and wp db import when MySQL/MariaDB binaries are unavailable.
Docstring Coverage ✅ Passed Docstring coverage is 90.91% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch copilot/fix-wp-db-query-fallback

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (3)
src/DB_Command.php (1)

1977-1997: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Uses a fresh connection per DDL call — fine for single calls, costly in clean()'s loop.

run_query() now routes every run_query() caller through this method when the binary is unavailable, including clean(), which calls run_query() once per table (Lines 235-244). Each call opens and closes a brand-new mysqli connection just to run one DROP TABLE. For large installs with many tables this multiplies connection overhead unnecessarily.

♻️ Suggested direction

Accept an optional existing mysqli connection (or expose a connection-reuse variant) so callers that issue many DDL statements in a loop (like clean()) can reuse a single connection instead of opening one per statement.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/DB_Command.php` around lines 1977 - 1997, Update run_query_via_mysqli()
to accept an optional existing mysqli connection, creating and closing its own
connection only when one is not provided. Modify clean() to obtain one reusable
connection before its table-drop loop and pass it to each run_query_via_mysqli()
call, then close it after the loop while preserving existing error handling.
features/db-query.feature (1)

158-183: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Scenario correctly matches the fallback's debug message; consider adding SQL-mode-compat coverage.

This scenario is well-formed and the previously requested env PATH=... fix is already applied. Given the critical SQL-mode-compatibility gap identified in wpdb_query()/wpdb_import() (src/DB_Command.php), consider adding a fallback scenario analogous to "wp db query adapts the SQL mode by default without a separate mode probe" (Lines 100-113) but using the fake-bin technique, so a fix to that gap is actually exercised by CI.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@features/db-query.feature` around lines 158 - 183, Add a fake-bin fallback
scenario in features/db-query.feature that verifies wp db query adapts SQL mode
by default without a separate mode probe, using the same PATH override and
unavailable mysql/mariadb binaries as the current scenario. Ensure the
assertions exercise the compatibility behavior implemented by
DB_Command::wpdb_query() or DB_Command::wpdb_import(), including the expected
query result and debug output.
features/db-import.feature (1)

72-99: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Scenario structure is correct but doesn't reproduce the known regression.

This test imports a plain export on what is likely a lenient-SQL-mode CI database, so it wouldn't fail even with the missing SQL-mode-compat statement in wpdb_import() (see src/DB_Command.php). Consider mirroring "wp db import loads a dump containing legacy zero-date values" (Lines 323-344) but through the fake-bin/mysqli fallback path, since that's the exact scenario that previously surfaced "Invalid default value for 'comment_date'" per the PR history.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@features/db-import.feature` around lines 72 - 99, The fallback scenario
should import a dump containing legacy zero-date values so it exercises the
SQL-mode compatibility logic in DB_Command::wpdb_import(), rather than importing
a plain export. Adapt the existing fake-bin/mysql and fake-bin/mariadb
unavailable setup to use the zero-date dump content and retain assertions for
successful import and the fallback warning.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/DB_Command.php`:
- Around line 2593-2629: Apply get_sql_mode_compat_statement( $assoc_args ) to
the fallback connection before executing SQL in both src/DB_Command.php lines
2593-2629 within wpdb_import() and lines 2535-2583 within wpdb_query(); each
site requires the same compatibility statement before the split statements or
query runs.
- Around line 2008-2013: Update the run_query() fallback around
is_mysql_binary_available() so database creation does not route through
run_query_via_mysqli() without a selected database. Exclude the create()
operation from this fallback, preserving the existing mysqli behavior for other
queries, or explicitly align the command documentation and design with
supporting create() through that path.
- Around line 2481-2524: Update get_db_connection() to use the command’s
effective CLI database username and password values, including --dbuser and
--dbpass overrides, instead of always reading DB_USER and DB_PASSWORD. Handle
mysqli connection exceptions so failures reach the existing WP_CLI::error()
path, and ensure fallback query operations similarly tolerate or catch mysqli
exceptions before relying on connect_errno/query checks.

---

Nitpick comments:
In `@features/db-import.feature`:
- Around line 72-99: The fallback scenario should import a dump containing
legacy zero-date values so it exercises the SQL-mode compatibility logic in
DB_Command::wpdb_import(), rather than importing a plain export. Adapt the
existing fake-bin/mysql and fake-bin/mariadb unavailable setup to use the
zero-date dump content and retain assertions for successful import and the
fallback warning.

In `@features/db-query.feature`:
- Around line 158-183: Add a fake-bin fallback scenario in
features/db-query.feature that verifies wp db query adapts SQL mode by default
without a separate mode probe, using the same PATH override and unavailable
mysql/mariadb binaries as the current scenario. Ensure the assertions exercise
the compatibility behavior implemented by DB_Command::wpdb_query() or
DB_Command::wpdb_import(), including the expected query result and debug output.

In `@src/DB_Command.php`:
- Around line 1977-1997: Update run_query_via_mysqli() to accept an optional
existing mysqli connection, creating and closing its own connection only when
one is not provided. Modify clean() to obtain one reusable connection before its
table-drop loop and pass it to each run_query_via_mysqli() call, then close it
after the loop while preserving existing error handling.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 147dfdef-c9e6-4d69-9dda-8223e5fcbc7f

📥 Commits

Reviewing files that changed from the base of the PR and between 4f7635d and 6051f7f.

📒 Files selected for processing (4)
  • features/db-import.feature
  • features/db-query.feature
  • features/db.feature
  • src/DB_Command.php

Comment thread src/DB_Command.php Outdated
Comment thread src/DB_Command.php
Comment thread src/DB_Command.php
@swissspidy swissspidy changed the title Add wpdb fallback for wp db query, wp db import, wp db drop, and wp db reset when mysql/mariadb binary is unavailable Add wpdb fallback for wp db query and wp db import when mysql/mariadb binary is unavailable Jul 22, 2026
@swissspidy
swissspidy force-pushed the copilot/fix-wp-db-query-fallback branch from a7665f9 to 30dfd4f Compare July 22, 2026 14:12
@swissspidy
swissspidy marked this pull request as ready for review July 22, 2026 14:19
@swissspidy
swissspidy requested a review from a team as a code owner July 22, 2026 14:19

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/DB_Command.php (1)

602-619: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Document the wpdb-fallback behavior and its limitations in the query/import docblocks. Both commands gained a significant new code path (automatic fallback to $wpdb when the mysql/mariadb binary is unavailable) with real behavioral differences from the mysql-client path, but neither docblock mentions it.

  • src/DB_Command.php#L602-L619: add a note to query()'s docblock that when the mysql/mariadb binary is unavailable, the query runs through $wpdb instead, --dbuser/--dbpass/other mysql-specific options are not honored in that mode, and multi-statement input (e.g. wp db query < file.sql) is not currently supported via this fallback.
  • src/DB_Command.php#L955-L977: add a note to import()'s docblock that the wpdb fallback ignores --dbuser/--dbpass, and that dumps relying on custom DELIMITER blocks (stored routines/triggers) or server-level DDL are not supported by the fallback.

As per path instructions, **/*.php: "Update relevant inline code documentation when changes affect user-facing functionality."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/DB_Command.php` around lines 602 - 619, Update the query() docblock at
src/DB_Command.php lines 602-619 to document that unavailable mysql/mariadb
binaries trigger execution through $wpdb, mysql-specific options including
--dbuser and --dbpass are ignored, and multi-statement input is unsupported.
Update the import() docblock at src/DB_Command.php lines 955-977 to document
that the fallback ignores --dbuser/--dbpass and does not support custom
DELIMITER blocks or server-level DDL.

Source: Coding guidelines

♻️ Duplicate comments (2)
src/DB_Command.php (2)

2501-2507: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

--dbuser/--dbpass (and other connection overrides) are silently ignored by the wpdb fallback.

maybe_load_wpdb() always instantiates wpdb with the raw DB_USER/DB_PASSWORD/DB_HOST constants, with no way to honor --dbuser/--dbpass passed to wp db query/wp db import. This is the same underlying gap flagged on the previous mysqli-based implementation (get_db_connection() reading DB_USER/DB_PASSWORD directly), just carried into the new wpdb-based fallback.

🛠️ Suggested direction
-	protected function maybe_load_wpdb() {
+	protected function maybe_load_wpdb( $assoc_args = [] ) {
 		...
-			$wpdb = new wpdb( DB_USER, DB_PASSWORD, DB_NAME, DB_HOST );
+			$dbuser = Utils\get_flag_value( $assoc_args, 'dbuser', DB_USER );
+			$dbpass = Utils\get_flag_value( $assoc_args, 'dbpass', DB_PASSWORD );
+			$wpdb   = new wpdb( $dbuser, $dbpass, DB_NAME, DB_HOST );

and pass $assoc_args through from wpdb_query()/wpdb_import().

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/DB_Command.php` around lines 2501 - 2507, Update maybe_load_wpdb() to
accept the connection override arguments and use them when constructing the
fallback wpdb instance instead of always using DB_USER, DB_PASSWORD, DB_NAME,
and DB_HOST directly. Pass $assoc_args through from wpdb_query() and
wpdb_import(), preserving the constants as defaults when overrides are absent.

2585-2606: 🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win

SQL-mode compatibility statement is missing from both wpdb fallback executors. Neither wpdb_import() nor wpdb_query() applies get_sql_mode_compat_statement( $assoc_args ) on the fallback connection, unlike the CLI path's --init-command. This is the same root cause previously flagged, and matches the PR history's reported persisting "Invalid default value for 'comment_date'" import failure across WordPress versions.

  • src/DB_Command.php#L2585-L2606: in wpdb_import(), run $this->get_sql_mode_compat_statement( $assoc_args ) via $wpdb->query() before executing the split statements.
  • src/DB_Command.php#L2519-L2575: in wpdb_query(), run the same compatibility statement before executing $query.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/DB_Command.php` around lines 2585 - 2606, Apply the SQL-mode
compatibility statement to both fallback executors before running SQL: in
src/DB_Command.php lines 2585-2606, update wpdb_import() to execute
get_sql_mode_compat_statement( $assoc_args ) through $wpdb->query() before the
split statements; likewise in src/DB_Command.php lines 2519-2575, update
wpdb_query() to execute the same statement before $query.
🧹 Nitpick comments (1)
src/DB_Command.php (1)

2656-2751: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Statement splitter doesn't handle custom DELIMITER blocks (stored routines/triggers).

split_sql_statements() correctly tracks quotes/comments/conditional comments, but has no concept of a custom DELIMITER (e.g. DELIMITER ;; used by dumps with --routines/--triggers). Any embedded ; inside such a routine body would be split as a statement boundary, corrupting the routine definition. Given --skip-optimization aside, this is a real gap for non-trivial dumps, though likely acceptable if this limitation is documented (see the docblock consolidation comment) rather than fixed outright, since the fallback already explicitly doesn't support server-level DDL.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/DB_Command.php` around lines 2656 - 2751, Update split_sql_statements()
to recognize DELIMITER directives and use the configured delimiter instead of
treating every semicolon inside stored routine or trigger definitions as a
statement boundary. Preserve existing quote, comment, and conditional-comment
handling, and ensure DELIMITER directives themselves are not included in
returned SQL statements; if this scope is intentionally unsupported, document
that limitation in the associated method docblock instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/DB_Command.php`:
- Around line 2519-2575: Update wpdb_query() to pass the input through the
existing split_sql_statements() helper and execute each resulting statement
individually, matching the iteration behavior used by wpdb_import(). Preserve
per-statement handling for modifying queries, errors, affected-row success
messages, result headers, and rows, while ensuring all statements from piped SQL
are processed.
- Around line 2519-2526: Update wpdb_query() to suppress wpdb errors during
query execution, mirroring the behavior in wpdb_import(). Capture the previous
suppression state, call $wpdb->suppress_errors(true) before executing the query,
and restore the captured state before every return path and when the method
completes.
- Around line 2633-2645: Update the transaction cleanup in the import flow
around the $wpdb query calls and its enclosing method so failures from COMMIT or
any settings-restore query are detected and propagated to the caller. Ensure
import() does not print its success message when cleanup fails, while preserving
normal success behavior when all queries complete successfully.
- Around line 2480-2493: Update the wpdb bootstrap required-files list near
$required_files to include WordPress's wp-includes/functions.php before loading
dependent files. Keep the existing require_once loop and file existence checks
unchanged so wpdb::print_error() and get_caller() can safely resolve
wp_debug_backtrace_summary().

---

Outside diff comments:
In `@src/DB_Command.php`:
- Around line 602-619: Update the query() docblock at src/DB_Command.php lines
602-619 to document that unavailable mysql/mariadb binaries trigger execution
through $wpdb, mysql-specific options including --dbuser and --dbpass are
ignored, and multi-statement input is unsupported. Update the import() docblock
at src/DB_Command.php lines 955-977 to document that the fallback ignores
--dbuser/--dbpass and does not support custom DELIMITER blocks or server-level
DDL.

---

Duplicate comments:
In `@src/DB_Command.php`:
- Around line 2501-2507: Update maybe_load_wpdb() to accept the connection
override arguments and use them when constructing the fallback wpdb instance
instead of always using DB_USER, DB_PASSWORD, DB_NAME, and DB_HOST directly.
Pass $assoc_args through from wpdb_query() and wpdb_import(), preserving the
constants as defaults when overrides are absent.
- Around line 2585-2606: Apply the SQL-mode compatibility statement to both
fallback executors before running SQL: in src/DB_Command.php lines 2585-2606,
update wpdb_import() to execute get_sql_mode_compat_statement( $assoc_args )
through $wpdb->query() before the split statements; likewise in
src/DB_Command.php lines 2519-2575, update wpdb_query() to execute the same
statement before $query.

---

Nitpick comments:
In `@src/DB_Command.php`:
- Around line 2656-2751: Update split_sql_statements() to recognize DELIMITER
directives and use the configured delimiter instead of treating every semicolon
inside stored routine or trigger definitions as a statement boundary. Preserve
existing quote, comment, and conditional-comment handling, and ensure DELIMITER
directives themselves are not included in returned SQL statements; if this scope
is intentionally unsupported, document that limitation in the associated method
docblock instead.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5a2840be-db42-4924-83dc-02975a8da892

📥 Commits

Reviewing files that changed from the base of the PR and between 6051f7f and 30dfd4f.

📒 Files selected for processing (3)
  • features/db-import.feature
  • features/db.feature
  • src/DB_Command.php
💤 Files with no reviewable changes (1)
  • features/db.feature
🚧 Files skipped from review as they are similar to previous changes (1)
  • features/db-import.feature

Comment thread src/DB_Command.php
Comment on lines +2480 to +2493
// Load prerequisite WordPress files if not already loaded.
$required_files = [
ABSPATH . WPINC . '/load.php',
ABSPATH . WPINC . '/compat.php',
ABSPATH . WPINC . '/plugin.php',
ABSPATH . WPINC . '/class-wp-error.php',
$wpdb_file,
];

foreach ( $required_files as $required_file ) {
if ( file_exists( $required_file ) ) {
require_once $required_file;
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate and inspect maybe_load_wpdb implementation and nearby context.
rg -n "maybe_load_wpdb|required_files|class-wp-error|load\.php|compat\.php|plugin\.php|functions\.php" src/DB_Command.php
printf '\n---\n'
sed -n '2440,2515p' src/DB_Command.php

printf '\n===\n'

# Search for direct references to helpers implicated by the review comment.
rg -n "_doing_it_wrong\(|is_multisite\(" src/DB_Command.php

Repository: wp-cli/db-command

Length of output: 2962


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Search repository for any local compatibility handling around wpdb/core bootstrap.
rg -n "maybe_load_wpdb|wpdb_file|wp-includes/functions\.php|_doing_it_wrong\(|is_multisite\(" .

Repository: wp-cli/db-command

Length of output: 543


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the WordPress core version references vendored or mentioned in the repo, if any.
git ls-files | rg '(^|/)wp-includes/|(^|/)wordpress/|(^|/)tests?/.*wordpress|wp-cli'

Repository: wp-cli/db-command

Length of output: 166


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Identify whether the repo includes tests or docs discussing maybe_load_wpdb failure modes.
rg -n "maybe_load_wpdb|fallback reported|WordPress core files|wpdb" .

Repository: wp-cli/db-command

Length of output: 9836


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- DB_Command.php excerpt ---'
nl -ba src/DB_Command.php | sed -n '2468,2505p'

echo
echo '--- search for helper calls in DB_Command.php ---'
rg -n "_doing_it_wrong\(|is_multisite\(|wpdb->prepare|get_results\(|query\(" src/DB_Command.php

echo
echo '--- search for related bootstrap / core-file mentions in repo ---'
rg -n "functions\.php|load\.php|compat\.php|plugin\.php|class-wp-error\.php|maybe_load_wpdb|wpdb_file" .

echo
echo '--- repo file inventory for tests/docs mentioning wpdb or core bootstrap ---'
git ls-files | rg '(test|spec|doc|readme|md)$|DB_Command|wpdb|wordpress'

Repository: wp-cli/db-command

Length of output: 226


🌐 Web query:

WordPress wpdb::prepare _doing_it_wrong functions.php load.php is_multisite class-wpdb.php

💡 Result:

In WordPress, the _doing_it_wrong function is used to notify developers that a specific function or method is being called incorrectly [1]. When you see an error related to wpdb::prepare and _doing_it_wrong, it indicates that the wpdb::prepare method—found in wp-includes/class-wpdb.php—has been used in a way that violates safety or API standards [2][3]. Common causes for this error include: 1. Missing Placeholders: The most frequent cause is calling wpdb::prepare with a query string that contains no placeholders (e.g., %s, %d, %f) [2][4]. WordPress requires that any query passed to this method must include at least one placeholder to ensure the data is properly escaped [2][5]. 2. Insufficient Arguments: The method expects a matching argument for every placeholder present in the query [6][3]. Providing fewer arguments than there are placeholders will trigger this notice [5][6]. 3. Mismatched Placeholders/Arguments: If a query contains multiple placeholders but the passed arguments do not align correctly (or if an array is used incorrectly), the system flags the usage [6][3]. Regarding the other components you mentioned: - wp-includes/functions.php: This is the file where the _doing_it_wrong function itself is defined [1][7]. It handles triggering the notification (often via the doing_it_wrong_run action or by outputting a warning in debug mode) [1]. - wp-includes/load.php: This file contains low-level functions required early in the WordPress lifecycle, such as is_multisite [8][9]. While not directly responsible for the wpdb::prepare logic, it contains helper functions like wp_load_translations_early which may be called by wpdb::prepare to ensure the _doing_it_wrong message is translatable [2][3]. - is_multisite: This function determines whether a WordPress site is running in a Multisite configuration [8]. It is defined in wp-includes/load.php [8]. It is not directly related to the logic of wpdb::prepare placeholders, but both are core components often involved in the early boot process of WordPress [3][8]. To resolve a "Function wpdb::prepare was called incorrectly" error, review the SQL query string passed as the first argument to ensure it contains at least one placeholder, and verify that every placeholder has a corresponding, correctly formatted argument [4]. You can use WP_DEBUG to display these notices during development [1].

Citations:


🌐 Web query:

site:github.com/WordPress/WordPress class-wpdb.php _doing_it_wrong functions.php

💡 Result:

The _doing_it_wrong function calls found within class-wpdb.php are triggered when the $wpdb database class detects improper usage of its methods, particularly wpdb::prepare or database escaping functions [1][2][3]. These warnings are intended for developers to help identify and fix insecure or technically incorrect code during development [1][3]. Common scenarios that trigger these warnings include: 1. Improper use of wpdb::prepare: - Missing placeholders: If a query passed to wpdb::prepare does not contain any placeholders (e.g., %), it triggers a warning because it suggests the query might not be properly sanitized [1][4][3]. - Unsupported value types: If the arguments passed to wpdb::prepare are not scalar or null (e.g., passing an object or array where a string or integer is expected), a warning is issued [5][6][3]. - Placeholder mismatch: If the number of provided arguments does not match the placeholders in the query string, a warning is triggered to prevent potential errors [1][4][3]. 2. Database escaping issues: - Missing database connection: The _real_escape method triggers a _doing_it_wrong warning if it is called before a database connection has been established, indicating an attempt to escape data without the necessary context [2][7][6]. When these warnings occur, they are typically logged to the debug.log file if WP_DEBUG and WP_DEBUG_LOG are enabled. To resolve them, you should examine the stack trace provided in the error message to locate the specific code (often in your theme's functions.php or a plugin file) that is calling the $wpdb method incorrectly and update it to follow WordPress coding standards, such as ensuring all SQL queries use proper placeholders [1][3].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the SQLite bootstrap because it may show which core helpers wpdb actually needs.
sed -n '540,590p' src/DB_Command_SQLite.php

Repository: wp-cli/db-command

Length of output: 1510


🌐 Web query:

WordPress wpdb wp_debug_backtrace_summary functions.php class-wpdb.php query print_error

💡 Result:

In WordPress, the relationship between wpdb, wp_debug_backtrace_summary, and print_error is central to database debugging. wp_debug_backtrace_summary is a utility function located in wp-includes/functions.php that generates a comma-separated string (or array) of function names in the current backtrace [1][2][3]. It is designed to provide context for debugging by showing the call stack up to the current point [3]. The wpdb class leverages this function for internal diagnostics [4][5]: 1. wpdb::get_caller: This internal method calls wp_debug_backtrace_summary(CLASS) to retrieve the name of the function(s) that triggered a specific database query [4]. 2. wpdb::print_error: When a database error occurs, this method uses wpdb::get_caller to identify where the offending query originated [6]. It then includes this "made by [caller]" information in the database error log entry [6]. Key debugging components: - wpdb::print_error(string $str = ''): This method prints or logs SQL/database errors [6]. If database errors are suppressed (via $wpdb->suppress_errors), it returns false [6]. If not, it logs the error message (including the caller's identity) via error_log [6]. On non-multisite installations, it also attempts to display the error directly on the screen if $wpdb->show_errors is enabled [6]. - SAVEQUERIES: If this constant is set to true in wp-config.php, WordPress tracks all queries in the global $wpdb->queries array [7][8]. This log includes the query SQL, execution time, and a comma-separated list of calling functions (generated using the same logic as the backtrace tools) [7]. In summary, wp_debug_backtrace_summary provides the trace data, which wpdb::get_caller extracts for use by wpdb::print_error to create detailed, debuggable error messages that identify exactly which part of your code initiated a failed query [4][6].

Citations:


Load functions.php in the wpdb bootstrap load.php already covers is_multisite(), but wpdb::print_error()/get_caller() rely on wp_debug_backtrace_summary() from wp-includes/functions.php, so a query failure can still fatal without it.

🧰 Tools
🪛 OpenGrep (1.25.0)

[ERROR] 2491-2491: Dynamic file path passed to include/require. This can lead to local or remote file inclusion. Use a fixed allowlist of paths.

(coderabbit.file-inclusion.php-dynamic-include)

🪛 PHPStan (2.2.5)

[error] 2482-2482: Constant ABSPATH not found.
Learn more at https://phpstan.org/user-guide/discovering-symbols

(constant.notFound)


[error] 2483-2483: Constant ABSPATH not found.
Learn more at https://phpstan.org/user-guide/discovering-symbols

(constant.notFound)


[error] 2484-2484: Constant ABSPATH not found.
Learn more at https://phpstan.org/user-guide/discovering-symbols

(constant.notFound)


[error] 2485-2485: Constant ABSPATH not found.
Learn more at https://phpstan.org/user-guide/discovering-symbols

(constant.notFound)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/DB_Command.php` around lines 2480 - 2493, Update the wpdb bootstrap
required-files list near $required_files to include WordPress's
wp-includes/functions.php before loading dependent files. Keep the existing
require_once loop and file existence checks unchanged so wpdb::print_error() and
get_caller() can safely resolve wp_debug_backtrace_summary().

Comment thread src/DB_Command.php
Comment on lines +2519 to +2526
protected function wpdb_query( $query, $assoc_args = [] ) {
$this->maybe_load_wpdb();
global $wpdb;

if ( ! isset( $wpdb ) || ! ( $wpdb instanceof wpdb ) ) {
WP_CLI::error( 'WordPress database (wpdb) is not available. Please install MySQL or MariaDB client tools.' );
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

wpdb_query() doesn't suppress wpdb's default error output, unlike wpdb_import().

wpdb_import() wraps execution in $wpdb->suppress_errors( true ), but wpdb_query() never does. With WordPress's default wpdb::$show_errors = true, a failing query can trigger wpdb's own PHP warning/HTML error output before WP_CLI::error() prints the clean message, leaking noisy output into the CLI.

🛠️ Proposed fix
 	protected function wpdb_query( $query, $assoc_args = [] ) {
 		$this->maybe_load_wpdb();
 		global $wpdb;

 		if ( ! isset( $wpdb ) || ! ( $wpdb instanceof wpdb ) ) {
 			WP_CLI::error( 'WordPress database (wpdb) is not available. Please install MySQL or MariaDB client tools.' );
 		}

+		$suppress = $wpdb->suppress_errors( true );
+
 		$skip_column_names      = Utils\get_flag_value( $assoc_args, 'skip-column-names', false );

(and restore $wpdb->suppress_errors( $suppress ); before each return path / at the end of the method, mirroring wpdb_import().)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/DB_Command.php` around lines 2519 - 2526, Update wpdb_query() to suppress
wpdb errors during query execution, mirroring the behavior in wpdb_import().
Capture the previous suppression state, call $wpdb->suppress_errors(true) before
executing the query, and restore the captured state before every return path and
when the method completes.

Comment thread src/DB_Command.php
Comment on lines +2519 to +2575
protected function wpdb_query( $query, $assoc_args = [] ) {
$this->maybe_load_wpdb();
global $wpdb;

if ( ! isset( $wpdb ) || ! ( $wpdb instanceof wpdb ) ) {
WP_CLI::error( 'WordPress database (wpdb) is not available. Please install MySQL or MariaDB client tools.' );
}

$skip_column_names = Utils\get_flag_value( $assoc_args, 'skip-column-names', false );
$is_row_modifying_query = (bool) preg_match( '/\b(UPDATE|DELETE|INSERT|REPLACE(?!\s*\()|LOAD DATA)\b/i', $query );

if ( $is_row_modifying_query ) {
// phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared
$affected_rows = $wpdb->query( $query );
if ( false === $affected_rows ) {
// phpcs:ignore WordPress.WP.AlternativeFunctions.strip_tags_strip_tags
WP_CLI::error( 'Query failed: ' . strip_tags( $wpdb->last_error ) );
}
WP_CLI::success( "Query succeeded. Rows affected: {$affected_rows}" );
} else {
// phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared
$results = $wpdb->get_results( $query, ARRAY_A );

if ( $wpdb->last_error ) {
// phpcs:ignore WordPress.WP.AlternativeFunctions.strip_tags_strip_tags
WP_CLI::error( 'Query failed: ' . strip_tags( $wpdb->last_error ) );
}

if ( empty( $results ) ) {
return;
}

$headers = array_keys( $results[0] );
if ( ! $skip_column_names && ! empty( $headers ) ) {
WP_CLI::line( implode( "\t", $headers ) );
}
foreach ( $results as $row ) {
WP_CLI::line(
implode(
"\t",
array_map(
static function ( $v ) {
if ( null === $v ) {
return 'NULL';
}
if ( is_scalar( $v ) ) {
return (string) $v;
}
return '';
},
array_values( $row )
)
)
);
}
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

wpdb_query() doesn't split multi-statement SQL, unlike wpdb_import().

The command's own docs show piping a file into wp db query ($ wp db query < debug.sql), which commonly contains multiple ;-terminated statements. wpdb_query() passes the entire string straight to a single $wpdb->query( $query ) / $wpdb->get_results( $query, ARRAY_A ) call. WordPress's wpdb connects without MYSQLI_CLIENT_MULTI_STATEMENTS, so a multi-statement string will typically fail or silently execute only the first statement — unlike the real mysql binary path, and unlike wpdb_import(), which already reuses split_sql_statements() for exactly this reason.

🛠️ Suggested direction

Reuse split_sql_statements() in wpdb_query() and iterate over the resulting statements the same way wpdb_import() does, aggregating/echoing results as each statement executes.

🧰 Tools
🪛 ast-grep (0.44.1)

[error] 2531-2531: Prevent SQL queries built from unsanitized input
Context: $wpdb->query( $query )
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').

(sql-injection-php)

🪛 PHPStan (2.2.5)

[error] 2540-2540: Constant ARRAY_A not found.
Learn more at https://phpstan.org/user-guide/discovering-symbols

(constant.notFound)


[error] 2551-2551: Parameter #1 $array of function array_keys expects array, array|stdClass given.

(argument.type)


[error] 2569-2569: Parameter #1 $array of function array_values expects array, array|stdClass given.

(argument.type)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/DB_Command.php` around lines 2519 - 2575, Update wpdb_query() to pass the
input through the existing split_sql_statements() helper and execute each
resulting statement individually, matching the iteration behavior used by
wpdb_import(). Preserve per-statement handling for modifying queries, errors,
affected-row success messages, result headers, and rows, while ensuring all
statements from piped SQL are processed.

Comment thread src/DB_Command.php
Comment on lines +2633 to +2645
if ( ! $skip_optimization ) {
// phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared
$wpdb->query( 'COMMIT' );
// phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared
$wpdb->query( 'SET autocommit = 1' );
// phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared
$wpdb->query( 'SET unique_checks = 1' );
// phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared
$wpdb->query( 'SET foreign_key_checks = 1' );
}

$wpdb->suppress_errors( $suppress );
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win

COMMIT/settings-restore failures after import are not checked — the caller reports success anyway.

If $wpdb->query( 'COMMIT' ) fails (lock timeout, dropped connection, etc.), the function returns normally with no error signaled. import() then unconditionally prints Success: Imported from '...'., even though the transaction may not have been committed — risking a false "success" report and silent data loss for the whole import.

🛠️ Proposed fix
 		if ( ! $skip_optimization ) {
 			// phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared
-			$wpdb->query( 'COMMIT' );
+			$commit_result = $wpdb->query( 'COMMIT' );
+			if ( false === $commit_result ) {
+				$error = $wpdb->last_error;
+				$wpdb->suppress_errors( $suppress );
+				// phpcs:ignore WordPress.WP.AlternativeFunctions.strip_tags_strip_tags
+				WP_CLI::error( 'Import failed to commit: ' . strip_tags( $error ) );
+			}
 			// phpcs:ignore WordPress.DB.PreparedSQL.NotPrepared
 			$wpdb->query( 'SET autocommit = 1' );
🧰 Tools
🪛 ast-grep (0.44.1)

[error] 2634-2634: Prevent SQL queries built from unsanitized input
Context: $wpdb->query( 'COMMIT' )
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').

(sql-injection-php)


[error] 2636-2636: Prevent SQL queries built from unsanitized input
Context: $wpdb->query( 'SET autocommit = 1' )
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').

(sql-injection-php)


[error] 2638-2638: Prevent SQL queries built from unsanitized input
Context: $wpdb->query( 'SET unique_checks = 1' )
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').

(sql-injection-php)


[error] 2640-2640: Prevent SQL queries built from unsanitized input
Context: $wpdb->query( 'SET foreign_key_checks = 1' )
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').

(sql-injection-php)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/DB_Command.php` around lines 2633 - 2645, Update the transaction cleanup
in the import flow around the $wpdb query calls and its enclosing method so
failures from COMMIT or any settings-restore query are detected and propagated
to the caller. Ensure import() does not print its success message when cleanup
fails, while preserving normal success behavior when all queries complete
successfully.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

query shouldn't require mysql/mariadb cli tools

3 participants